Automated review: request changes

Two blocking issues are in the inline comments: a NULL row can now abort Damerau-Levenshtein before its null map is reapplied, and no regression reaches the BE paths changed by this patch. Please also update the PR metadata: trailing-NUL distance results deliberately change, so Release note: None, the unchecked Behavior changed item, and the empty test checklist do not accurately describe the PR.

Critical checkpoints:

• Goal and proof: non-null inputs consistently retain real trailing NUL bytes across Hamming, Levenshtein, and Damerau-Levenshtein, but mixed nullable blocks can fail before NULL is reapplied and no base-distinguishing test proves the intended behavior.
• Scope and clarity: the production change is small and focused on the two affected BE implementations.
• Concurrency and lifecycle: no shared mutable state, locks, threads, static initialization dependency, or special resource lifetime is introduced.
• Error, nullability, and parallel paths: Hamming's custom nullable paths skip NULL rows, but the shared Levenshtein/Damerau wrapper evaluates arbitrary nested NULL-row payloads and can surface a matrix-cap error; const/vector routes, aliases, and FE folding otherwise use complete values consistently.
• Configuration and compatibility: no configuration, protocol, serialized format, or FE-BE variable changes are introduced. Production CHAR storage read paths remove padding before expression evaluation, while STRING/VARCHAR lengths remain intact.
• Tests and results: blocking gap; existing tests cover ordinary branch shapes but never the changed trailing-NUL behavior or the mixed nullable error path, and this PR changes no generated result file.
• Performance and memory: removing repeated tail scans is favorable for non-null values and adds no allocation site, but arbitrary NULL-row payloads must not trigger avoidable distance-matrix work.
• Observability, persistence, transactions, and data writes: not applicable to this expression-only change.
• User focus: no additional focus was provided; the whole PR was reviewed.

Review was static as required by the action prompt; no build or test command was run.

Automated follow-up review: changes still requested

Round 1 converged with two normal full-review subagents and a separate risk-focused subagent. All three returned NO_NEW_VALUABLE_FINDINGS: no new inline issue survives validation or duplicate suppression. The two existing P1 threads are still unresolved, non-outdated, and anchored to the current head:

• A mixed nullable block can evaluate a retained trailing-NUL nested payload before the null map is reapplied, so a semantically NULL row can abort Damerau-Levenshtein at its matrix cap.
• No regression currently reaches the changed BE paths with trailing-NUL data or proves that CHAR storage padding stays invisible.

Critical checkpoints:

• Goal and proof: the non-NULL execution paths consistently preserve legitimate trailing NUL bytes for Hamming, Levenshtein, and Damerau-Levenshtein, but the nullable failure and missing base-distinguishing regression mean the goal is not safely proven end to end.
• Scope and clarity: the production diff is small, focused, and limited to removing tail trimming in the two affected BE implementations.
• Concurrency: no threads, shared mutable state, locks, atomics, or lock ordering are involved.
• Lifecycle and initialization: no new static initialization dependency, ownership cycle, cache handle, or special resource lifetime is introduced.
• Error handling, nullability, and parallel paths: Hamming skips NULL rows before evaluation and propagates failures; the shared Levenshtein/Damerau default nullable wrapper does not. Vector/vector, both constant orientations, ASCII/UTF-8 branches, aliases, and FE folding otherwise use complete values consistently.
• Configuration and compatibility: no configuration, FE-BE variable, protocol, function symbol, or persisted/storage format changes are introduced. No rolling-upgrade compatibility branch is required for this scalar semantic correction. Current CHAR direct, dictionary, fallback, and statistics read paths strip physical padding before expression evaluation, while VARCHAR/STRING preserve trailing NUL.
• Conditional logic: no new special condition is added; the existing Hamming null checks have concrete nullable-row paths and remain consistent across branches.
• Tests and results: blocking gap. Existing tests cover ordinary dispatch shapes but contain no table-backed trailing NUL, no CHAR comparison for this behavior, and no mixed nullable retained-payload case; this PR changes no generated result file. Literal folding cannot prove the BE fix because Nereids already counts U+0000.
• Performance and memory: removing repeated tail scans is beneficial on non-NULL inputs and adds no allocation site, but the nullable path can now perform avoidable quadratic work or fail the existing matrix limit on a result that must be NULL.
• Observability: no new operational path needs logs or metrics.
• Persistence, transactions, data writes, and crash recovery: not applicable to this expression-only change.
• Other review state: the existing review-body request to align Release note: None, the unchecked behavior-change item, and the empty test checklist with the deliberate user-visible change remains applicable.
• User focus: I treated unrelated legacy concerns as out of scope, as requested. The two retained blockers are directly tied to this trim removal: one is newly exposed by preserving the payload, and the other is the missing proof for the behavior this PR changes. No additional legacy issue is being raised.

This was a static review as required by the action prompt; no build or test command was run in this review environment.

LGTM